Port andor new ad format #724
Conversation
| def _get_trigger_mode(self, trigger: DetectorTrigger) -> Andor2TriggerMode: | ||
| supported_trigger_types = { | ||
| DetectorTrigger.INTERNAL: Andor2TriggerMode.INTERNAL, | ||
| DetectorTrigger.EDGE_TRIGGER: Andor2TriggerMode.EXT_TRIGGER, | ||
| } | ||
| if trigger not in supported_trigger_types: | ||
| raise ValueError( | ||
| f"{self.__class__.__name__} only supports the following trigger " | ||
| f"types: {supported_trigger_types} but was asked to " | ||
| f"use {trigger}" | ||
| ) | ||
| return supported_trigger_types[trigger] |
There was a problem hiding this comment.
Nit: If we can't make a new TriggerInfo for Andor, then it might nicer to replace this code with a from_detector_trigger(cls, trigger: DetectorTrigger) -> Andor2TriggerMode method on Andor2TriggerMode.
| def get_deadtime(self, exposure: float | None) -> float: | ||
| return _MIN_DEAD_TIME + (exposure or 0) | ||
|
|
There was a problem hiding this comment.
I think i did a bad thing when I first made it, may be the better way to get the dead time is to use the accumulate period rather than exposure time, which we can read from this signal: BL99P-EA-DET-03:CAM:AndorAccumulatePeriod_RBV ?
There was a problem hiding this comment.
Except this is a sync method so we can't read PVs here...
There was a problem hiding this comment.
Just out of interest, if I do want the deadtime to be from a readback channel, what is the best way to go about it?
There was a problem hiding this comment.
The answer is either:
- We keep the ability to validate plans in advance, the we cannot touch hardware to calculate this number and have to duplicate the calculation of deadtime that's in the SDK into our code
- We abandon the ability to validate plans in advance, then we can make this an async method and use the value of that PV after exposure time has been set
@callumforrester do we want to open this box?
…rt-andor-new-ad-format
|
@Relm-Arrowny will you have a chance to try this on a real Andor please? |
sure, I will try this tomorrow or whenever it is free. |
|
I fell over on the first step, I could not connect due to mismatch DataType |
|
@Relm-Arrowny I've pushed another change that moves the superclass init before the overrides to |
Connected and did a few scans with it, as far as I can tell it is working better than ever, thanks. |
| @@ -16,23 +16,30 @@ class Andor2TriggerMode(StrictEnum): | |||
| SOFTWARE = "Software" | |||
|
|
|||
|
|
|||
| class ImageMode(StrictEnum): | |||
| class Andor2ImageMode(StrictEnum): | |||
There was a problem hiding this comment.
Could we make this one subclass the ADBase one and add FAST_KINETICS to it?
| self.trigger_mode = epics_signal_rw(Andor2TriggerMode, prefix + "TriggerMode") | ||
| self.data_type = epics_signal_r(ADBaseDataType, prefix + "DataType_RBV") | ||
| self.andor_accumulate_period = epics_signal_r( | ||
| float, prefix + "AndorAccumulatePeriod_RBV" | ||
| ) | ||
| self.image_mode = epics_signal_rw_rbv(ImageMode, prefix + "ImageMode") | ||
| super().__init__(prefix, name=name) | ||
| self.image_mode = epics_signal_rw_rbv(Andor2ImageMode, prefix + "ImageMode") |
There was a problem hiding this comment.
This seems a relatively common pattern, overriding the image mode to be a subset of what the baseclass provides. We have SubsetEnum already, should we have SupersetEnum too? Or we could drop to string in the baseclass...
Takes #679 and adds modifications to work with the changes to the new
ADclass structure. I don't have an Andor to test with, so would be good if we could test with the device.